Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix OAUTH_CUSTOM_FULL mode #527

Merged
merged 2 commits into from
Jan 21, 2024
Merged

Fix OAUTH_CUSTOM_FULL mode #527

merged 2 commits into from
Jan 21, 2024

Conversation

MarvinSchenkel
Copy link
Contributor

This PR adds the missing AuthType.OAUTH_CUSTOM_FULL mode when calculating the headers. We (Music Assistant) use our own web-based OAuth process to retrieve an acces token. In this scenario we also provide the other required headers to interact with Youtube Music.

sigma67
sigma67 reviewed Jan 21, 2024
View reviewed changes
Copy link
Owner

@sigma67 sigma67 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this already be supported via

https://github.com/sigma67/ytmusicapi/blob/main/ytmusicapi/ytmusic.py#L190-L191

If not, can we have a test as well that covers the difference and ensures it won't break again in the future?

@MarvinSchenkel
Copy link
Contributor Author

Good point! In that case I would need to make some changes to this line, as the token is None for full custom oauth:

ytmusicapi/ytmusicapi/ytmusic.py

Line 215 in 9170197

self._headers["authorization"] = self._token.as_auth()
. This currently throws an error.

@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024

Yeah, is it enough to just not set authorization if self.auth_type == AuthType.OAUTH_CUSTOM_FULL: ?

@MarvinSchenkel
Copy link
Contributor Author

Yeah, is it enough to just not set authorization if self.auth_type == AuthType.OAUTH_CUSTOM_FULL: ?

Absolutely, fixed that

@sigma67 sigma67 merged commit 294fb81 into sigma67:master Jan 21, 2024
2 of 3 checks passed
@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024

@MarvinSchenkel please submit to main next time, master is no longer used

@MarvinSchenkel
Copy link
Contributor Author

@MarvinSchenkel please submit to main next time, master is no longer used

Sorry I wasn't aware. My existing fork was still pointing to master.

@sigma67
Copy link
Owner

sigma67 commented Jan 21, 2024 •
edited
Loading

I deleted the master branch in this repo so this can't happen anymore 👍

(don't worry I'll push your commit shortly)

sigma67 pushed a commit that referenced this pull request Jan 21, 2024
@MarvinSchenkel @sigma67
Fix OAUTH_CUSTOM_FULL mode (#527)
8831090 
* Fix OAUTH_CUSTOM_FULL mode

* Fix OAUTH_CUSTOM_FULL mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sigma67 sigma67 sigma67 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MarvinSchenkel @sigma67